fix(transform): make the mid-turn release valve actually detect injections - #273
Conversation
…tions
`hasNewerRealUserMessage` tested `$.synthetic` on the MESSAGE row, but
OpenCode persists that flag on the PART row's data — never on the message.
Measured on a live opencode.db: message-level `$.synthetic` matches 0 rows,
part-level matches 11,320. The predicate was therefore always true, so every
synthetic injection (Channel-2 nudges, s2s frames, subagent notifications)
counted as a real user message and released the mid-turn lock while a turn
was still accumulating tool calls.
The comment above the return asserted the opposite ("message.info.synthetic,
which is the top-level $.synthetic field in the message table's data JSON"),
and the existing test fixtured `synthetic: true` on the message row — a shape
production never produces. Comment and test agreed with each other, which is
why the inert filter survived.
Replace it with a part join. A part is machine-generated iff it carries
`synthetic: true` OR a `metadata.marker.kind`; a user message is injected iff
it HAS parts AND EVERY part is machine-generated.
Both clauses are load-bearing:
- Marker parts (s2s inbox lines, subagent-message notifications, steer/cancel
notices) are deliberately NON-synthetic so the TUI renders them as visible
system-event lines, and are tagged structurally via `metadata.marker.kind`
instead. Testing `synthetic` alone misses them.
- ALL-parts rather than ANY: an @mention puts a synthetic `agent` part on a
genuine operator prompt, so ANY-semantics would classify live human turns as
injected and suppress the lock on real input — the inverse bug, and worse.
- The `EXISTS (… FROM part …)` clause is the vacuous-ALL fence: a partless user
message satisfies "every part is machine-generated" trivially and must count
as real. Partless user rows do occur (4 of 19,004 on the box measured).
Rewrote the misleading test to fixture the part-level shape, and added cases
for marker parts, @mention prompts, partless messages, and mixed
marker-plus-operator-text. The synthetic-part and marker-part cases are
red-verified against the unfixed query.
|
CI is red on this PR, and it's pre-existing on The three All three fail at the Why it doesn't reproduce locally: the Check job's lint gate is version-unstable. Line 48 of
Same tree, same command. The new errors are rules that tightened between patches — Practical consequence for the repo, independent of this PR: Local gates on this branch, for the record: |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/plugin/src/hooks/magic-context/read-session-db.ts">
<violation number="1" location="packages/plugin/src/hooks/magic-context/read-session-db.ts:152">
P2: The whole fix now depends on two external-schema assumptions: (1) that `synthetic` is persisted on the `part` row rather than the `message` row, and (2) that system-event (inbox/s2s) parts expose their kind at `data.metadata.marker.kind`. If the real OpenCode part shape differs from `metadata.marker.kind` — e.g. the marker data is stored directly on the part instead of nested under `metadata` — then marker parts would not be detected as machine-generated, and a user message whose only part is a marker would be misclassified as real, releasing the mid-turn lock (exactly the inverse bug the PR set out to fix). The new tests currently construct fixtures inside `insertPart(...)` using the same `metadata: { marker: { kind: ... } }` assumption the code reads, so they are self-confirming and cannot catch a mismatch with the real persisted shape. The PR itself already flags that `marker.kind` is 'not currently documented'. Please verify the marker path against a real captured OpenCode part row and consider adding a fixture sourced from actual persisted data rather than the assumed shape.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| SELECT 1 FROM part p | ||
| WHERE p.message_id = m.id | ||
| AND COALESCE(json_extract(p.data, '$.synthetic'), 0) NOT IN (1, 'true') | ||
| AND json_extract(p.data, '$.metadata.marker.kind') IS NULL |
There was a problem hiding this comment.
P2: The whole fix now depends on two external-schema assumptions: (1) that synthetic is persisted on the part row rather than the message row, and (2) that system-event (inbox/s2s) parts expose their kind at data.metadata.marker.kind. If the real OpenCode part shape differs from metadata.marker.kind — e.g. the marker data is stored directly on the part instead of nested under metadata — then marker parts would not be detected as machine-generated, and a user message whose only part is a marker would be misclassified as real, releasing the mid-turn lock (exactly the inverse bug the PR set out to fix). The new tests currently construct fixtures inside insertPart(...) using the same metadata: { marker: { kind: ... } } assumption the code reads, so they are self-confirming and cannot catch a mismatch with the real persisted shape. The PR itself already flags that marker.kind is 'not currently documented'. Please verify the marker path against a real captured OpenCode part row and consider adding a fixture sourced from actual persisted data rather than the assumed shape.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/plugin/src/hooks/magic-context/read-session-db.ts, line 152:
<comment>The whole fix now depends on two external-schema assumptions: (1) that `synthetic` is persisted on the `part` row rather than the `message` row, and (2) that system-event (inbox/s2s) parts expose their kind at `data.metadata.marker.kind`. If the real OpenCode part shape differs from `metadata.marker.kind` — e.g. the marker data is stored directly on the part instead of nested under `metadata` — then marker parts would not be detected as machine-generated, and a user message whose only part is a marker would be misclassified as real, releasing the mid-turn lock (exactly the inverse bug the PR set out to fix). The new tests currently construct fixtures inside `insertPart(...)` using the same `metadata: { marker: { kind: ... } }` assumption the code reads, so they are self-confirming and cannot catch a mismatch with the real persisted shape. The PR itself already flags that `marker.kind` is 'not currently documented'. Please verify the marker path against a real captured OpenCode part row and consider adding a fixture sourced from actual persisted data rather than the assumed shape.</comment>
<file context>
@@ -139,18 +139,35 @@ function hasNewerRealUserMessage(
+ SELECT 1 FROM part p
+ WHERE p.message_id = m.id
+ AND COALESCE(json_extract(p.data, '$.synthetic'), 0) NOT IN (1, 'true')
+ AND json_extract(p.data, '$.metadata.marker.kind') IS NULL
+ )
+ )
</file context>
There was a problem hiding this comment.
The self-confirming-fixture criticism is correct on method, and I've addressed it by sourcing the fixtures from real data. Both assumptions verified against a live opencode.db rather than reasoned about.
Assumption 1 — synthetic on part, not message. This is what the PR is fixing, and it's measurable: json_extract(data,'$.synthetic') on the message table returns 0 rows across the entire DB, versus 11,320 rows on part. The old message-level predicate could never match, which is why the valve was inert.
Assumption 2 — metadata.marker.kind. Present and populated: 740 inbox, 168 interrupt, 805 message. Verbatim part.data.metadata from real rows:
{"marker":{"kind":"inbox","from":"…","sessionId":"…"}}
{"marker":{"kind":"interrupt","intent":"abort","origin":"parent"}}
{"marker":{"kind":"message","peer":"subagent","expectReply":false}}The fixtures now use these shapes (session id and peer name replaced with placeholders) instead of the invented { marker: { kind } }. That closes the gap you identified two ways: the path is confirmed against persisted data, and because the real shapes carry sibling fields beside kind, the tests now also prove the predicate isn't accidentally depending on marker having exactly one key. Coverage widened from inbox alone to all three kinds.
On the inverse-bug risk specifically: I'd rather not rest on fixtures alone, so it's worth noting the ALL-parts semantics gives a second line of defense. For a marker path mismatch to release the lock, the message would need every part to be undetectable — and I checked what happens under ANY-semantics instead: 534 messages would flip, 525 of them genuine human prompts containing @mentions (an @mention appends a synthetic agent part to a real prompt). That asymmetry is why ALL is the correct quantifier here, and it's now covered by an explicit @mention test.
Worth flagging one thing I could not verify: marker.kind is real in persisted data but is not in a published schema I can find, so it remains an undocumented shape we depend on. If it moves, the failure is silent. I'd take a stable discriminator upstream if one is offered.
…id-turn valve
`hasNewerRealUserMessage` classified a user message as REAL whenever any part
was non-synthetic and carried no `metadata.marker.kind`. Parts flagged
`ignored: true` satisfied both conditions, so an ignored-only message released
the mid-turn lock and allowed a tail rewrite while a tool turn was still
running.
Measured on a live opencode.db: 102 ignored parts, all on user messages, none
synthetic, none carrying a marker — and every one of those messages was
ignored-ONLY, so all 102 released the lock. They are plugin status
notifications (bodies like "## Claude Routing Status", "## Claude Quotas").
What settles the classification is opencode's own serializer, not our
inference. message-v2.ts:206 keeps a user text part only when
`!part.ignored && part.text !== ""`, so an ignored part never reaches the
model. A message whose parts are all ignored contributes nothing to the prompt
and therefore cannot be a real user turn.
Adds a third machine-generated clause to the inner NOT EXISTS witness, using
the same COALESCE truthiness convention as the synthetic clause (both `1` and
`'true'` count).
Tests: the marker fixtures previously used an invented
`metadata: { marker: { kind } }` shape — self-confirming, since the code reads
the same assumption. They now use shapes captured verbatim from real persisted
part rows (live counts: inbox 740, interrupt 168, message 805), including the
sibling fields beside `kind` (`from`/`sessionId`, `intent`/`origin`,
`peer`/`expectReply`), which also proves the predicate does not depend on
`marker` having exactly one key. Session id and peer name are placeholders.
Five cases added: ignored-only keeps the lock; ignored part alongside genuine
operator text still RELEASES it (the guard against suppressing a legitimate
release); numeric `ignored: 1`; and interrupt + message marker kinds.
RED verified by reverting the query alone — the two ignored cases fail
(23 pass / 2 fail) and pass again with the clause restored (25/0).
|
Merged — and verified your core claim against a live opencode.db before merging: zero rows carry |
…alesce + migration 45, permission regex pair, persisted deny verdict (v73), #273 part-shape pins, home-git containment gate
3 participants
The bug
hasNewerRealUserMessage(read-session-db.ts:133) decides whether a newer real user message exists after the latest assistant turn —isMidTurnFromOpenCodeDbuses it to release the mid-turn lock. It filtered on$.syntheticon the message row:OpenCode persists
syntheticon the part row's data, never on the message row. Measured on a liveopencode.db:json_extract(data,'$.synthetic')onmessagejson_extract(data,'$.synthetic')onpartSo the predicate is always true. Every synthetic injection — Channel-2 ceiling nudges, s2s frames, subagent-message notifications — counts as a real user message and releases the mid-turn lock while a turn is still accumulating tool calls. The lock exists so the transform never rewrites bytes mid-turn, so this quietly gives up that protection.
Two things kept it hidden: the comment above the return asserted the opposite ("OpenCode persists promptAsync/channel-2 synthetic prompts as
message.info.synthetic, which is the top-level$.syntheticfield in the message table's data JSON"), and the existing test fixturedsynthetic: trueon the message row — a shape production never produces, which passes only because the filter reads that field. Comment and test agreed with each other and both were wrong.The fix
A part join. A part is machine-generated iff it carries
synthetic: trueOR ametadata.marker.kind; a user message is injected iff it HAS parts AND EVERY part is machine-generated.All three pieces are load-bearing, and each was found by measurement rather than reasoning:
1.
marker.kindis required. Marker parts — s2s inbox lines, subagent-message notifications, steer/cancel notices — are deliberately non-synthetic so the TUI renders them as visible system-event lines (packages/tui/src/routes/session/index.tsx≈1431-1441:!x.synthetic && metadata.marker.kind !== undefined). They're tagged structurally instead. Live counts: 1,778 such parts (message847,inbox683,interrupt248). Testingsyntheticalone misses all of them.2. ALL-parts, not ANY. 1,813 user messages carry a synthetic part and a non-synthetic companion. Of those, 525 are genuine operator prompts that used an
@mention— the mention adds a syntheticagentpart beside real human text. ANY-semantics would classify those live human turns as injected and suppress the lock on real input: the inverse bug, and worse than the original.3. The
EXISTS (… FROM part …)clause is the vacuous-ALL fence. A partless user message satisfies "every part is machine-generated" trivially and must count as real. Partless user rows do occur — 4 of 19,004 on the box measured — so this is a live case, not a theoretical one.The misleading comment is replaced with one that states where
syntheticactually lives and why each clause exists.Tests
Rewrote the existing synthetic test to fixture the part-level shape (it still expects
true— still mid-turn), and added four cases:truemetadata.marker.kind: "inbox")truemarker.kindclause@mentionoperator prompt (real text + syntheticagentpart)falsefalsefalseThe synthetic-part and marker-part cases are red-verified against the unfixed query:
Restoring the fix: 20 pass / 0 fail.
Gates
read-session-db.test.tsbun testmasterbaseline 3353; +4 net)tsc --noEmitCI note:
Check (plugin),Check (pi-plugin)andCheck (dashboard)are currently failing onmasteritself (7af5961d) with lint errors in files this PR doesn't touch —compartment-chunk-embedding.ts,storage-git-commits.ts,render-mural.test.ts,embedding-local.test.tsand others. This branch inherits that red; it doesn't add to it. Happy to include the lint cleanup here if you'd rather, but it seemed wrong to bundle unrelated fixes into a behavioral change.Note on scope
I deliberately did not propose tagging the marker/companion parts
syntheticto make a single-flag test work — that would erase every ✉/⊘ system-event line from the operator's transcript, per the TUI predicate above. Separately worth flagging for maintainers:metadata.marker.kindappears to be the real "machine-generated" discriminator but isn't documented as such, so a consumer reaching for the obvioussyntheticflag gets the classification wrong in both directions. A short note in the docs, or a dedicatedsystemGeneratedfield, would prevent the next consumer repeating this. Happy to file that separately if useful.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes mid-turn lock detection so synthetic, marker-only, and ignored-only user messages no longer release the lock. This prevents premature unlocks while tool calls are still running.
synthetic,metadata.marker.kind, orignored).ignored: 1, and interrupt/message markers.Written for commit 1ce9b36. Summary will update on new commits.
Greptile Summary
The PR corrects mid-turn release detection by classifying user messages from their associated parts instead of a nonexistent message-level synthetic flag.
@mentionmessage shapes.Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Newer user message] --> B{Has parts?} B -- No --> R[Real user message: release lock] B -- Yes --> C{Every part synthetic, marker-tagged, or ignored?} C -- Yes --> I[Injected message: keep lock] C -- No --> RReviews (2): Last reviewed commit: "fix(transform): treat ignored-only user ..." | Re-trigger Greptile